Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

Support options passed to .open #660

Merged
merged 1 commit into from
Jun 28, 2019
Merged

Support options passed to .open #660

merged 1 commit into from
Jun 28, 2019

Conversation

achingbrain
Copy link
Contributor

LevelDown supports an optional options argument passed the db.open method. Currently LevelUp fails if options are passed to .open as it tries to invoke them as the callback so this PR adds support for that.

By and large you don't call .open directly but if you do (or an intermediate module does as in my case) LevelUp should probably honour the api.

@achingbrain
Support options passed to .open
8e8764a 
LevelDown supports an [optional `options` argument](https://github.com/Level/abstract-leveldown#dbopenoptions-callback)
passed the `db.open` method.  Currently LevelUp fails if options are
passed to `.open` so this PR adds support for that.
@vweevers
Copy link
Member

vweevers commented Jun 25, 2019 •
edited
Loading

Options that you pass to the levelup constructor are passed on to db.open().

And personally I'm hoping we can move away from db.open() options altogether. It's much easier to "compose" (combine) modules if they take their options in the constructor, like encoding-down, level-js and a few others do.

@achingbrain
Copy link
Contributor Author

Options that you pass to the levelup constructor are passed on to db.open()

That behaviour is maintained.

I'm hoping we can move away from db.open() options altogether

Sounds like a good plan, but if the goal here is to support the same API as abstract-leveldown but with added streams, then support for passing options to db.open should be in there until it's removed from abstract-leveldown itself.

@vweevers
Copy link
Member

but if the goal here is to support the same API as abstract-leveldown but with added streams

That's not - or has not been - the goal. However there is a long-term goal to merge levelup functionality into abstract-leveldown. So on second thought, it's good to bring the two interfaces closer together :) I'm in

@vweevers vweevers added enhancement New feature or request semver-minor New features that are backward compatible labels Jun 26, 2019
@vweevers vweevers self-requested a review June 26, 2019 10:14
@vweevers vweevers merged commit 9071230 into Level:master Jun 28, 2019
@vweevers
Copy link
Member

This needs a follow-up; browser tests are failing on the use of arrow functions.

vweevers added a commit that referenced this pull request Jun 28, 2019
@vweevers
Replace arrow functions (follow-up for #660)
7c5a0b1
vweevers added a commit that referenced this pull request Jun 28, 2019
@vweevers
Replace arrow functions (follow-up for #660) (#662)
9318f99
@vweevers
Copy link
Member

4.1.0

@vweevers vweevers mentioned this pull request Sep 18, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vweevers vweevers vweevers approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request semver-minor New features that are backward compatible
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@achingbrain @vweevers